Rethinking the beep.Streamer interfaces
#170
Conversation
They don't add much value if the implementing structs are only going to proxy the Close call to the source io.Reader. Besides, other streamers don't call Close() on their source streamers, so the user needs to keep track of closing the streamer themselves anyway.
MarkKremer
force-pushed
the
update-streamer-interfaces
branch
from
822668e to
48a5d9b
Compare
|
2 cents: However, I could see usecases where we want to pass the ReadCloser into a streamer and keep the streamer around outside of the calling scope (which in the new examples would free the underlying resource and cause a panic). I think that keeping the StreamCloser interface is good and we could make it a bit smarter such that the decoder just takes a Reader, then checks to see if it's a *Closer when decoder.Close is called, otherwise, it's a no-op. If that makes sense. |
|
You make some good points. My argumentation for this interface is that Beep's composites need to be configured separately anyway. E.g. if you have a streamer, volume and ctrl node, you'll need to keep hold of them so that you can close, change volume and pause the stream. I think it's a good programming pattern to keep them in a struct to encapsulate that part of the audio business logic. So what's an extra I invite further opinions but I'll probably hold this PR until after I'm further with the docs and/or my music player projects so I'll have more experience. |
beep.StreamCloserandbeep.StreamSeekCloser. These functions do not seem to add much value because the decoders that implement them only proxy the call to their sourceio.Reader. Besides, otherbeep.Streamersdo not callClose()on their source streamers, so the user has to keep track of when to close the decoder or reader themselves regardless. The result of this is that allio.Reader's padded to aDecode()function must be closed manually.midi.NewSoundFont()also doesn't close the suppliedio.Readeranymore.TODO